Skip to content

perf: reduce clock reads per request from 9 to 3 - #2016

Open
jkaczman wants to merge 2 commits into
jk-shrink-movesfrom
jk-request-clock-reads
Open

jkaczman wants to merge 2 commits into
jk-shrink-movesfrom
jk-request-clock-reads

Conversation

@jkaczman

Copy link
Copy Markdown
Contributor

Previously, we read the clock 9 times per request (there were a couple other ones that I missed in #1890 from not looking at the flamegraph).

Now we read it 3 times (+ tokio's): once at the start of the request, once for the checkout timeout in Pool::get, and once in Server::read on ReadyForQuery

Stats, new request and all of our timers reuse prior ones now.

Removes ~650 instructions and ~2.4% of user cycles per transaction in pooler bench

@jkaczman
jkaczman added this pull request to stack #2012 November 11, 2026 18:02
self.statement_executed
}

pub(crate) fn take_last_ready_for_query(&mut self) -> Option<Instant> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd rename it to something more self-documenting, maybe take_last_query_exec_time?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, maybe we could make it pub(in crate::backend) to make sure someone doesn't accidentally use it incorrectly? Not sure what kind of side effect it would have; previously, all server timings were self-contained, so an outside caller couldn't break them.

@levkk

levkk commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

A case can be made here to fork Tokio to add a once-per-runtime-tick Instant cache. Might help with code legibility. Not a blocker obviously (forking Tokio is a full time job), but something to consider in the future maybe.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants